The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: up1512001 <[email protected]>
Co-authored-by: fabiankaegy <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: t-hamano <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Actually @up1512001 looks like the test failure here in CI is legit. Seems like some of the test snapshots need to be updated

@getdave As of #41744 we have a block support for aria-label that gets used in a few places. It uses the ariaLabel attribute under the hood. It does not provide a UI for it however. So I'd be okay with changing this PR to only add the attribute but not add a UI just yet if we are concerned about adding it.

Maybe @richtabor has an opinion as I know he has spent some time trying to clean up the Advanced panel

I agree. Let's solve this using the method from #41744.

Let's be consistent with the other approaches then. I'm curious as to why we didn't provide a UI in the Group block.

@getdave
This is what the other issue said:

This implementation does NOT add a UI for the aria-label. A visible option would have the potential to confuse users, and could cause more harm than good if they enter the wrong thing or misunderstand the purpose.
However, allowing users & theme-authors to manually define an aria-label in their templates is something that can significantly enhance the a11y of themes, without exposing any option to users. Furthermore, implementing this as a hook will allow us to implement a UI if at some point in the future we choose to do so.

And agreed 👍 @up1512001 can you make that update? :)

@getdave This is what the other issue said:

Thanks. I should have looked at that more carefully. That seems entirely logical. We can always add a control later if we need.

I'm a bit confused, since the Navigation block already supports ariaLabel as a block support, developer should be able to override aria-label via the code editor etc, right?

I'm a bit confused, since the Navigation block already supports ariaLabel as a block support, developer should be able to override aria-label via the code editor etc, right?

This is dynamic block and inside editor user only sees like

<!-- wp:navigation {"ref":51} /-->

that's why ariaLabel help to provide any meaningful explanation to menu like this

<!-- wp:navigation {"ref":51,"ariaLabel":"global header"} /-->

Let me know if I am missing anything 🙇‍♂️

So does that mean that the current Navigation block ariaLabel block support doesn't work unless we explicitly add the ariaLabel attribute? Looking at #54418, it seems to work as expected with just the ariaLabel block support.

Or is this PR trying to change how aria-label attributes are handled server-side?

Sorry if I misunderstood something. I'm trying to understand what this PR is trying to achieve.

You're correct, @t-hamano! The aria-label support was introduced in #54418, but it stopped working properly after that.

To get it working again, I think we just need to fix the logic in the WP_Navigation_Block_Renderer::get_navigation_name method.

@Mamaduka added early return in WP_Navigation_Block_Renderer::get_navigation_name if ariaLable is not empty.